Add 7.1 migration guide#2774
Conversation
|
This pull request needs "/ok-to-test" from an authorized committer. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated the migration guide to cover Membrane 7.1: document removals and renames, YAML/OpenAPI key changes, element-level inline configuration, chain/choose behavior, and many before/after YAML examples for migrating configurations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/MIGRATION-GUIDE.md`:
- Line 77: Remove the stray tilde marker on line 77 in MIGRATION-GUIDE.md by
deleting the empty line that contains only the tilde (~) so the stray annotation
is removed and the document no longer has the unnecessary blank/marker line.
- Line 30: The code example has inconsistent indentation around the HTML comment
markers; remove the extra leading spaces before the string "<!-- now -->" so it
lines up with the "<!-- before -->" marker in the same example, ensuring both
comment lines have identical leading whitespace for visual consistency.
- Around line 399-426: Reword the explanation of the choose syntax change to
make clear that the "cases:" wrapper is removed and that both "case" and
"otherwise" are now direct list items under "choose:" (with "otherwise" required
to be the final item). Update the text that currently reads "The `otherwise` is
now an item of the `case` list" to say something like: "The `choose` structure
now uses a single list containing both `case` and `otherwise` items (without a
`cases:` wrapper). The `otherwise` must be the last item in this list." Ensure
the examples reflect this exact structure (change any `cases:` blocks to the new
direct-list form) and keep references to `choose`, `case`, `otherwise`, and
`cases:` so reviewers can locate the change.
🧹 Nitpick comments (2)
docs/MIGRATION-GUIDE.md (2)
260-260: Consider lowercasing "Attribute" for consistency."Attribute" is capitalized mid-sentence on line 260, which is inconsistent with standard technical documentation conventions. Unless these are specific type names in the codebase, consider using lowercase for better readability.
428-447: Consider clarifying the structural change beyond renaming.Line 430 states "Renamed
specstoopenapi," but the change is more than a simple rename. The structure is also flattened:
- Old:
specs:→ list of- openapi:objects- New:
openapi:→ direct list of configurationsConsider adding: "The
specskey is renamed toopenapi, and the nestedopenapi:wrapper for each list item is removed."This helps users understand they need to adjust the structure, not just rename the key.
… and configuration changes for 7.1
…les, and configuration changes for 7.1
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/MIGRATION-GUIDE.md`:
- Around line 354-355: The heading text has inconsistent capitalization: change
"Yaml" to "YAML" and make "Elements" lowercase ("elements") in the phrase "Yaml
configuration for Elements with exactly one configurable Attribute" so it reads
"YAML configuration for elements with exactly one configurable Attribute";
update that heading string accordingly.
🧹 Nitpick comments (2)
docs/MIGRATION-GUIDE.md (2)
215-217: Consider adding migration guidance for removed component and behavioral change.Two areas could benefit from additional detail:
- Line 215: The removed
MethodOverrideInterceptorlacks information about alternatives or workarounds. If no replacement exists, explicitly state that.- Line 217: The groovy interceptor content-type change is a breaking behavioral change. Consider adding a brief code example showing how to manually set the content type in the script.
277-296: Remove redundant "Before:" heading.Line 280 contains a Markdown heading "Before:" that is redundant because the YAML examples already include
# beforeand# nowinline comments. The heading adds no additional value and creates visual clutter.📝 Proposed cleanup
## OpenApi Renamed `specs` to `openapi`. -**Before:** ```yaml # before api:
…ptor changes, and removal updates for 7.1
….x migration steps, and expand YAML configuration examples
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/MIGRATION-GUIDE.md`:
- Around line 44-45: Update the migration guide line for `xmlSessionIdExtractor`
to hyphenate "language-based" so the description reads "Use language-based
session handling." Locate the table row containing the `xmlSessionIdExtractor`
entry and replace the phrase "Use language based session handling. See
examples/loadbalancing/4-session" with the hyphenated version while keeping the
reference to examples/loadbalancing/4-session unchanged.
- Around line 218-236: The "7.1" migration section mixes numbered and unnumbered
headings — make numbering consistent by dropping the numeric prefix in the "3.
Removed Interceptors and Plugins" heading and any other numbered subheadings
inside the 7.1 section so all subsection headings use the same style (e.g.,
change "## 3. Removed Interceptors and Plugins" to "## Removed Interceptors and
Plugins"); ensure the table and references (including the
`MethodOverrideInterceptor` row) remain unchanged and that other 7.1 subsections
follow the same non-numbered heading pattern.
Summary by CodeRabbit